Skip to content

Replace try! with ?.#36041

Merged
bors merged 13 commits into
rust-lang:masterfrom
ahmedcharles:try
Sep 14, 2016
Merged

Replace try! with ?.#36041
bors merged 13 commits into
rust-lang:masterfrom
ahmedcharles:try

Conversation

@ahmedcharles

Copy link
Copy Markdown
Contributor

No description provided.

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

@nrc

nrc commented Aug 28, 2016

Copy link
Copy Markdown
Member

lgtm, but just checking with @rust-lang/compiler that they approve

@eddyb

eddyb commented Aug 28, 2016

Copy link
Copy Markdown
Contributor

cc @jonathandturner Does this change anything libsyntax uses or might use?
cc @erickt @dtolnay Making sure syntex doesn't get blocked.

@nrc

nrc commented Aug 28, 2016

Copy link
Copy Markdown
Member

This is all internal stuff, linbsyntax is touched in #36037

@eddyb

eddyb commented Aug 28, 2016

Copy link
Copy Markdown
Contributor

@nrc I was asking mostly because of a potential move of libsyntax to the error code system used in rustc.

@ahmedcharles

Copy link
Copy Markdown
Contributor Author

I can move changes (if there are any) that would impact syntex to #36037, easily enough and then get this merged.

@dtolnay

dtolnay commented Aug 28, 2016

Copy link
Copy Markdown
Member

None of these affect syntex.

@nrc

nrc commented Aug 29, 2016

Copy link
Copy Markdown
Member

@bors: r+

@bors

bors commented Aug 29, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit 26296c4 has been approved by nrc

@nikomatsakis

Copy link
Copy Markdown
Contributor

👍

@bors

bors commented Sep 1, 2016

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #35718) made this pull request unmergeable. Please resolve the merge conflicts.

@ahmedcharles

Copy link
Copy Markdown
Contributor Author

I assume this needs to be approved again @nrc?

@bors

bors commented Sep 3, 2016

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #36227) made this pull request unmergeable. Please resolve the merge conflicts.

@nrc

nrc commented Sep 5, 2016

Copy link
Copy Markdown
Member

@ahmedcharles yes, and unfortunately it needs another rebase now. When you push new commits, it is a good idea to ping the reviewer in a comment since GitHub does not send notifications about new commits (thus why I missed this).

@dtolnay

dtolnay commented Sep 5, 2016

Copy link
Copy Markdown
Member

@nrc

GitHub does not send notifications about new commits (thus why I missed this).

It does if you check the "Pull Request pushes" box at https://github.com/settings/notifications:

selection_007

@sophiajt

sophiajt commented Sep 5, 2016

Copy link
Copy Markdown
Contributor

@dtolnay - if we checked that, for the amount of PRs we review, we'd likely swamp our inbox and still not see it, sadly

@ahmedcharles

Copy link
Copy Markdown
Contributor Author

@nrc Probably ready for another try.

@nrc

nrc commented Sep 9, 2016

Copy link
Copy Markdown
Member

@bors: r+

@bors

bors commented Sep 9, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit a51dc66 has been approved by nrc

@bors

bors commented Sep 9, 2016

Copy link
Copy Markdown
Collaborator

⌛ Testing commit a51dc66 with merge 97d8c0c...

@bors

bors commented Sep 9, 2016

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-win-msvc-64-cargotest

@alexcrichton

Copy link
Copy Markdown
Member

@bors: retry

On Fri, Sep 9, 2016 at 2:04 PM, bors notifications@github.com wrote:

💔 Test failed - auto-win-msvc-64-cargotest
https://buildbot.rust-lang.org/builders/auto-win-msvc-64-cargotest/builds/1774


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#36041 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95KX4xXyVGHZWsOyAUhE62f2G0Uz9ks5qocnhgaJpZM4JutjV
.

@bors

bors commented Sep 10, 2016

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-win-msvc-64-cargotest

@ahmedcharles

Copy link
Copy Markdown
Contributor Author

Seems like an intermittent failure @alexcrichton @nrc ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file does not look like it's actually part of this PR.

@ahmedcharles

Copy link
Copy Markdown
Contributor Author

The merge conflict issue is fixed. @nrc.

@nrc

nrc commented Sep 13, 2016

Copy link
Copy Markdown
Member

@bors: r+

@bors

bors commented Sep 13, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit 694d601 has been approved by nrc

@bors

bors commented Sep 14, 2016

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 694d601 with merge 389abeb...

@bors

bors commented Sep 14, 2016

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-win-msvc-64-opt-rustbuild

@alexcrichton

Copy link
Copy Markdown
Member

@bors: retry

On Tue, Sep 13, 2016 at 6:43 PM, bors notifications@github.com wrote:

💔 Test failed - auto-win-msvc-64-opt-rustbuild
https://buildbot.rust-lang.org/builders/auto-win-msvc-64-opt-rustbuild/builds/2482


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#36041 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95BZszGUfH0VDa7urOsQCCOQdy3Tdks5qp1E2gaJpZM4JutjV
.

bors added a commit that referenced this pull request Sep 14, 2016
@bors

bors commented Sep 14, 2016

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 694d601 with merge 739d571...

@bors bors merged commit 694d601 into rust-lang:master Sep 14, 2016
@ahmedcharles ahmedcharles deleted the try branch September 17, 2016 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants